Autocomplete: Implement context filter #3897
Conversation
philipp-spiess
changed the title
| @@ -55,6 +58,10 @@ const defaultOptions = { | |||
| } | |||
|
|
|||
| describe('ContextMixer', () => { | |||
| beforeEach(() => { | |||
| vi.spyOn(contextFiltersProvider, 'isUriAllowed').mockImplementation(() => Promise.resolve(true)) | |||
There was a problem hiding this comment.
@valerybugakov Do you think it reasonable to manually mock this method for all tests or should we try to mock it automatically for all tests? 🤔
There was a problem hiding this comment.
Do you mean for all tests in the codebase? In some cases, we mock the underlying logic only (e.g., isRepoNameIgnored or fetchContextFilters), so un-mocking the isUriAllowed function would be necessary, which is not ideal. WDYT?
There was a problem hiding this comment.
@valerybugakov Oh no I didn't mean we should add this mock automatically but rather just make you aware that with the current config, we will have to update all tests that call into the context filters to add this line on top. Seems reasonable I guess?
We're going to have UX treatment for ignored files (for example, banner in JetBrains) which explain what is going on. A log at the verbose log level would be good. |
Ah awesome, I also saw this as a P1 here now: #3604. If that's planned, I'll not be closing this one. Verbose log for autocomplete is a bit annoying (since it will fire on every keystroke). I'll add a simple guard so that repeated calls for the same URI don't log something, this should be good enough for now and only requires us to keep a string value stored and compared against. |
philipp-spiess
force-pushed
the
ps/call-filter-autocomplete
branch
from
72f895d
to
43e36d4
Compare
philipp-spiess
force-pushed
the
ps/call-filter-autocomplete
branch
from
b9e025a
to
1a025f3
Compare
Part of #3642
Part of #3604
This PR implements context filter honoring for Autocomplete. There are two types of sources we filter:
Test plan